Support ChangeLog for the new branch model [skip ci] - #13687
Conversation
1, Retrieve commit hashes for a release after enabling the new branch model. 2, Create a query to fetch pull request (PR) information from GitHub using commit hashes. 3, Support retrieving PR changelogs for both the old and new branch models. Signed-off-by: timl <timl@nvidia.com>
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR modernizes the changelog generation script to accommodate the repository's branch naming transition from branch-YY.MM to release/YY.MM, starting at version 25.12. The implementation adds a commit-based PR lookup path using git log and GitHub's GraphQL API, while preserving backward compatibility with the legacy branch-name query method. The logic pivots on a hardcoded cutoff (FROM_RELEASE = '25.12') and handles three scenarios: both releases after the cutoff (commit-based), mixed mode at the cutoff, and both before the cutoff (branch-based). The script now sorts releases in descending order to ensure git range syntax works correctly, since get_commits() expects the newest release first for constructing from_branch..to_branch ranges.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
| scripts/generate-changelog | 3/5 | Adds commit-based PR retrieval for new branch model, with hybrid logic handling three release scenarios and git log parsing. |
Confidence score: 3/5
- This PR introduces significant control-flow branching and external process calls (git, GraphQL), increasing the surface area for runtime errors and edge cases.
- Score reflects three concerns: line 325 incorrectly treats a sorted list as a set (will always take first element), line 250 silently takes only the first associated PR per commit (losing multi-PR commits), and line 255uses a bare except that swallows all exceptions including keyboard interrupts and network failures.
- Pay close attention to the
process_pr()function (lines 323-345) andget_pr_via_commits()(lines 244-258), as these contain the problematic set/list confusion, silent exception handling, and assumptions about commit-to-PR cardinality.
Sequence Diagram
sequenceDiagram
participant User
participant main
participant process_changelog
participant process_pr
participant get_commits
participant get_pr_via_commits
participant fetch
participant post
participant GitHub API
participant form_changelog
User->>main: "Execute script with --releases, --token, --path"
main->>main: "Parse arguments and validate GITHUB_TOKEN"
main->>process_changelog: "Process PRs for releases"
process_changelog->>process_pr: "Get PRs based on releases"
alt Current version > FROM_RELEASE (25.12)
process_pr->>get_commits: "Get commit hashes between release tags"
get_commits->>get_commits: "Execute git log for each release pair"
get_commits-->>process_pr: "Return ver_commits dict"
process_pr->>get_pr_via_commits: "Get PRs from commit hashes"
loop For each commit SHA
get_pr_via_commits->>post: "Query PR by commit SHA"
post->>GitHub API: "POST query_pr_by_commit with SHA"
GitHub API-->>post: "Return PR data"
post-->>get_pr_via_commits: "Return response"
get_pr_via_commits->>get_pr_via_commits: "Add PR to list if merged and unique"
end
get_pr_via_commits-->>process_pr: "Return PR list"
else Current version == FROM_RELEASE
process_pr->>get_commits: "Get commits for FROM_RELEASE"
get_commits-->>process_pr: "Return ver_commits"
process_pr->>get_pr_via_commits: "Get PRs from commits"
get_pr_via_commits-->>process_pr: "Return PR list"
process_pr->>fetch: "Fetch PRs for previous branch"
fetch->>post: "Query PRs with baseRefName"
post->>GitHub API: "POST query_pr"
GitHub API-->>post: "Return PR data"
post-->>fetch: "Return response"
fetch-->>process_pr: "Return additional PRs"
else Current version < FROM_RELEASE
loop For each release
process_pr->>fetch: "Fetch PRs for branch-{release}"
fetch->>post: "Query PRs with baseRefName"
post->>GitHub API: "POST query_pr"
GitHub API-->>post: "Return PR data"
post-->>fetch: "Return response"
fetch-->>process_pr: "Extend PR list"
end
end
process_pr-->>process_changelog: "Return all PRs"
process_changelog->>process_changelog: "Categorize PRs by project and labels"
process_changelog-->>main: "Update changelog dict with PRs"
main->>process_changelog: "Process Issues for releases"
process_changelog->>process_issue: "Get issues from last 3*N months"
process_issue->>fetch: "Fetch issues with since timestamp"
loop While hasNextPage
fetch->>post: "Query issues"
post->>GitHub API: "POST query_issue"
GitHub API-->>post: "Return issue data"
post-->>fetch: "Return response"
fetch->>fetch: "Append issues and update cursor"
end
fetch-->>process_issue: "Return all issues"
process_issue-->>process_changelog: "Return issues"
process_changelog->>process_changelog: "Categorize issues by project and labels"
process_changelog-->>main: "Update changelog dict with issues"
main->>form_changelog: "Generate markdown from changelog dict"
form_changelog->>form_changelog: "Sort projects, format sections"
form_changelog->>form_changelog: "Write to file"
form_changelog-->>main: "Changelog file created"
main-->>User: "Done"
1 file reviewed, 3 comments
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the changelog generation script to support a transition from branch-based releases to release-branch-based releases starting with version 25.12. The script now uses git commit history to find PRs for releases after 25.12, while continuing to use the GitHub API for earlier releases.
Key changes:
- Added logic to fetch PRs via git commit history for releases >= 25.12
- Implemented version-based branching strategy detection (branch-YY.MM vs release/YY.MM)
- Maintained backward compatibility for releases before 25.12
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: timl <timl@nvidia.com>
|
some follow updates could be taken care later:
|
|
build |
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Extended changelog generation to support the new branch model (release/YY.MM) starting from version 25.12, while maintaining backward compatibility with the old model (branch-YY.MM). Adds commit-based PR retrieval using GraphQL queries, and implements logic to determine which branch naming convention to use based on release versions.
Key changes:
- Added
get_commits()to retrieve commit hashes between release branches usinggit log - Added
get_pr_via_commits()to fetch PR metadata from GitHub API using commit SHAs - Modified
process_pr()to route to old vs new branch model based onFROM_RELEASEthreshold (25.12) - Added
get_prev_release_version()utility to calculate previous release version
Critical issues found:
- Logic bug in
process_pr()line 330: assumes all releases use new model when highest version >FROM_RELEASE, but list can contain older versions requiring old model - Bug in
get_commits()line 214: always usesorigin/release/{to_rel}prefix, even for releases beforeFROM_RELEASE - Type hint mismatches between function signatures and actual parameters
- Cherry-picked commit handling may select wrong PR (existing comment)
Confidence Score: 2/5
- This PR has critical logic bugs that will cause failures when generating changelogs for mixed old/new release versions
- Score reflects two critical logic bugs: (1)
process_pr()incorrectly routes all releases to new branch model when only the highest version exceedsFROM_RELEASE, causing git failures for older releases, and (2)get_commits()always usesrelease/prefix forto_branchregardless of version. These bugs will break changelog generation for common scenarios likereleases=['26.02', '25.10']. Additionally, there are type hint inconsistencies and the existing cherry-pick issue remains unresolved. - Critical attention needed for scripts/generate-changelog lines 214, 330-332 to fix branch model routing logic
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| scripts/generate-changelog | 2/5 | Added support for new release branch model (release/YY.MM), but has critical logic bug in process_pr that incorrectly assumes all releases use new model when highest version > FROM_RELEASE, and get_commits always uses release/ prefix for to_branch |
Sequence Diagram
sequenceDiagram
participant User
participant main
participant process_pr
participant get_commits
participant get_pr_via_commits
participant GitHub API
User->>main: --releases=26.02,25.12
main->>main: Sort releases descending → [26.02, 25.12]
main->>process_pr: releases=[26.02, 25.12]
alt current_ver > FROM_RELEASE (26.02 > 25.12)
process_pr->>get_commits: All releases
loop For each release
get_commits->>get_commits: Determine from/to branches
Note over get_commits: to_branch = origin/release/{ver}<br/>from_branch = origin/release/ or branch-
get_commits->>get_commits: git log from_branch..to_branch
end
get_commits-->>process_pr: ver_commits dict
process_pr->>get_pr_via_commits: ver_commits
loop For each commit
get_pr_via_commits->>GitHub API: query_pr_by_commit(sha)
GitHub API-->>get_pr_via_commits: PR info
get_pr_via_commits->>get_pr_via_commits: Deduplicate & filter
end
get_pr_via_commits-->>process_pr: PR list
else current_ver == FROM_RELEASE (25.12)
process_pr->>get_commits: {FROM_RELEASE}
process_pr->>get_pr_via_commits: commits for 25.12
process_pr->>GitHub API: fetch PRs for branch-{prev_ver}
GitHub API-->>process_pr: Old model PRs
else current_ver < FROM_RELEASE
loop For each release
process_pr->>GitHub API: fetch(baseRefName=branch-{rel})
GitHub API-->>process_pr: PRs
end
end
process_pr-->>main: Combined PR list
main->>main: Generate changelog markdown
1 file reviewed, 5 comments
| if current_ver_float > based_rel: | ||
| ver_commits = get_commits(releases) | ||
| pr = get_pr_via_commits(ver_commits, token) |
There was a problem hiding this comment.
logic: Logic assumes all releases use new branch model when current_ver > FROM_RELEASE, but releases list can contain versions ≤ FROM_RELEASE. Example: if releases=['26.02', '25.10'], it calls get_commits which tries origin/release/25.10 (doesn't exist, should be origin/branch-25.10). Consider filtering releases or checking each version individually.
| ver_commits = {} | ||
| count = len(rel_list) # descending version order assured | ||
| for i, to_rel in enumerate(rel_list): | ||
| to_branch = f"origin/release/{to_rel}" |
There was a problem hiding this comment.
logic: Always uses origin/release/{to_rel} even for releases before FROM_RELEASE. Should check if float(to_rel) >= float(FROM_RELEASE) before deciding branch name, similar to from_branch logic on lines 221-224.
| # Get the commit hashes between two branches or release tags. | ||
| # param releases: set of release versions, e.g. {'YY.MM2', 'YY.MM1'} | ||
| # return: dict of commit hashes, e.g. {YY.MM2: [sha1, sha2, ...], YY.MM1: [shaX, shaY, ...]} | ||
| def get_commits(releases: set): |
There was a problem hiding this comment.
style: Type hint says set but receives sorted list from line 435. Update to def get_commits(releases: list): or keep as set and handle sorting internally.
| # param ver_commits, e.g. {v1: [sha1, sha2, ...], v2: [shaX, shaY, ...]} | ||
| # param token: the token for the API | ||
| # return: list of PRs associated with the commit hashes, e.g. [{PR1 info}, {PR2 info}, ...] | ||
| def get_pr_via_commits(ver_commits: set, token: str): |
There was a problem hiding this comment.
syntax: Type hint says set but receives dict from get_commits. Should be def get_pr_via_commits(ver_commits: dict, token: str):
| ] | ||
|
|
||
| # Use check=True to raise exception if git fails, making errors explicit | ||
| result = subprocess.run(git_log_args, capture_output=True, text=True, check=True) |
There was a problem hiding this comment.
style: Git command will fail silently returning empty commits if branches don't exist. Consider checking branch existence first or handling the specific error case separately from other exceptions.
… the changelog Signed-off-by: timl <timl@nvidia.com>
|
build |
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Extends changelog generation to support the new branch model where release branches transition from branch-YY.MM to release/YY.MM starting from version 25.12.
Key Changes:
- Adds commit-hash-based PR retrieval using
get_commits()andget_pr_via_commits()for releases ≥ 25.12 - Implements hybrid approach in
process_pr()to handle old model (baseRefName queries), new model (commit-based), and transition release (25.12) - Adds
FROM_RELEASEconstant (25.12) to mark the transition point - Introduces
get_prev_release_version()to calculate 2-month-prior releases
Critical Issues Found:
- Line 214:
to_branchalways usesorigin/release/{to_rel}format, which will fail for mixed-version lists containing releases < 25.12 - Line 210: Converting sorted list back to unsorted list loses the descending order required by loop logic
- Line 325: Assumes
releases(typed as set) is ordered, but works only due to line 436's conversion to sorted list - Type inconsistencies between function signatures (expecting
set) and actual usage (passing sortedlist)
Confidence Score: 2/5
- PR has critical logic errors that will cause failures with mixed-version release lists
- The implementation has fundamental issues: (1) to_branch determination on line 214 doesn't check if to_rel uses old/new model, causing git commands to fail for pre-25.12 releases, (2) type inconsistencies where functions expect sets but receive lists, (3) ordering dependencies that break when converting sorted lists back to unordered structures. These will cause runtime failures when generating changelogs for mixed-version scenarios
- scripts/generate-changelog requires fixes to branch name logic, type consistency, and ordering preservation
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| scripts/generate-changelog | 2/5 | Adds new branch model support with commit-based PR retrieval, but has critical logic errors in branch name determination, type inconsistencies, and ordering assumptions |
Sequence Diagram
sequenceDiagram
participant User
participant main
participant process_pr
participant get_commits
participant get_pr_via_commits
participant GitHub API
User->>main: Call with releases (e.g., "26.02,25.12")
main->>main: Parse and sort releases descending
main->>process_pr: Pass sorted releases
alt current_ver > FROM_RELEASE (25.12)
process_pr->>get_commits: Get commits for all releases
loop For each release
get_commits->>get_commits: Determine to_branch (origin/release/YY.MM)
get_commits->>get_commits: Determine from_branch (check < FROM_RELEASE)
get_commits->>GitHub API: git log from_branch..to_branch
GitHub API-->>get_commits: Return commit hashes
end
get_commits-->>process_pr: Return ver_commits dict
process_pr->>get_pr_via_commits: Pass ver_commits and token
loop For each commit in each version
get_pr_via_commits->>GitHub API: Query PR by commit SHA
GitHub API-->>get_pr_via_commits: Return associated PRs
get_pr_via_commits->>get_pr_via_commits: Filter and deduplicate
end
get_pr_via_commits-->>process_pr: Return PR list
else current_ver == FROM_RELEASE
process_pr->>get_commits: Get commits for FROM_RELEASE only
process_pr->>get_pr_via_commits: Get PRs via commits
process_pr->>GitHub API: Fetch PRs for previous release (old model)
GitHub API-->>process_pr: Return PRs
else current_ver < FROM_RELEASE
loop For each release
process_pr->>GitHub API: Fetch PRs by baseRefName (branch-YY.MM)
GitHub API-->>process_pr: Return PRs
end
end
process_pr-->>main: Return all PRs
main->>main: Process and generate changelog
1 file reviewed, 3 comments
| ver_commits = {} | ||
| count = len(rel_list) # descending version order assured | ||
| for i, to_rel in enumerate(rel_list): | ||
| to_branch = f"origin/release/{to_rel}" |
There was a problem hiding this comment.
logic: to_branch assumes new branch model for all releases in the list, but mixed release versions (e.g., ['26.02', '25.10']) will fail for older releases
| to_branch = f"origin/release/{to_rel}" | |
| to_rel_float = float(to_rel) | |
| if to_rel_float < float(FROM_RELEASE): | |
| to_branch = f"origin/branch-{to_rel}" | |
| else: | |
| to_branch = f"origin/release/{to_rel}" |
| # param releases: set of release versions, e.g. {'YY.MM2', 'YY.MM1'} | ||
| # return: dict of commit hashes, e.g. {YY.MM2: [sha1, sha2, ...], YY.MM1: [shaX, shaY, ...]} | ||
| def get_commits(releases: set): | ||
| rel_list = list(releases) |
There was a problem hiding this comment.
logic: Converting sorted list to unsorted list loses the ordering that line 436 established. Ordering matters for the loop logic on lines 216-219
| rel_list = list(releases) | |
| rel_list = releases # Already sorted in descending order |
| def process_pr(releases: set, token: str): | ||
| pr = [] | ||
| for rel in releases: | ||
| current_ver = list(releases)[0] |
There was a problem hiding this comment.
logic: list(releases)[0] assumes ordering but releases is typed as set (unordered). Works due to line 436 conversion but type hint is incorrect
| current_ver = list(releases)[0] | |
| current_ver = releases[0] # releases is already a sorted list |
|
bad base ref |


1, Retrieve commit hashes for a release after enabling the new branch model.
2, Create a query to fetch pull request (PR) information from GitHub using commit hashes.
3, Support retrieving PR ChangeLogs for both the old and new branch models.